Rewrite push_to_hub to use upload_files #18366

sgugger · 2022-07-29T18:42:56Z

What does this PR do?

As asked in #18299 this PR rewrites the PushToHubMixin completely to use upload_file behind the scenes. This is breaking since there is no more git repository involved, so while the user might have been left with a proper repository in the past, this is not the case anymore. The trade-off is that there is no need to clone anything now, so the upload should be faster.

Another breaking change is to default use_temp_files to None now, which then defaults to True if there is a local folder of the same name, False otherwise. This felt way more natural with this change, but we can discuss and revisit if needed. For the rest, normally backward compatibility is ensured with proper deprecation warnings when needed.

Using push_to_hub=True in save_pretrained is adapted to this rewrite but with 0 breaking change normally.

You can see in the tests how this simplifies a lot of things at the end of the day, so I'm quite happy with the final API.

Oh and one last change I made is to move the code specific to TensorFlow models in the TF modeling utils file.

HuggingFaceDocBuilderDev · 2022-07-29T18:53:41Z

The documentation is not available anymore as the PR was closed or merged.

julien-c

also cc'ing @Wauplin for visibility (and potential review)

julien-c · 2022-07-29T20:24:37Z

src/transformers/utils/hub.py

+            url = upload_file(
+                path_or_fileobj=os.path.join(working_dir, file),
+                path_in_repo=file,
+                repo_id=repo_id,
+                token=token,
+                commit_message=commit_message,
+            )


why not upload all files in one commit, using create_commit (or upload_folder)?

Yes, this could be updated to leverage either of the two so that a single commit is done.

The create_commit docs are available here.

The upload_folder docs are available here.

If using upload_folder, you won't need to track modified files with their timestamps: if the files have the same name as others on the Hub and contain the same values, they won't get pushed (so if you have local checkpoints that have already been pushed it won't push them again, only verify if they're on the Hub).

Added some comments to use upload_folder instead, it seems like it's an easy enough drop-in replacement

upload_folder is not ideal here as we may be in a folder where the user has some other stuff (like training artifacts) they didn't explicitly asked us to push. But create_commit should work.

(I was asked to use upload_file looks like my instructions were not correct 😝 )

seems like you have to take your "instructions" from the right persons=)

LysandreJik

Thanks for your PR @sgugger!

It looks great to me, and uses a non-deprecated API which is great. I've added pointers for the switch from upload_file to upload_folder, which is a drop-in replacement.

I've tested that all tests pass locally, the only change that will need to be done is in the commit message attributed to the upload.

LysandreJik · 2022-08-01T10:22:50Z

docs/source/en/model_sharing.mdx

->>> pt_model.push_to_hub("my-awesome-model", organization="my-awesome-org")
+>>> pt_model.push_to_hub("my-awesome-org/my-awesome-model")


Very welcome change

LysandreJik · 2022-08-01T10:24:35Z

src/transformers/utils/hub.py

+                repo_id = f"{organization}/{repo_id}"
+
+        token = HfFolder.get_token() if use_auth_token is True else use_auth_token
+        url = create_repo(repo_id=repo_id, token=token, private=private, exist_ok=True)


Nice, this prepares transformers for huggingface_hub version 0.10.0 (was previously using deprecated behavior)

LysandreJik · 2022-08-01T10:26:08Z

src/transformers/utils/hub.py

+            url = upload_file(
+                path_or_fileobj=os.path.join(working_dir, file),
+                path_in_repo=file,
+                repo_id=repo_id,
+                token=token,
+                commit_message=commit_message,
+            )


Yes, this could be updated to leverage either of the two so that a single commit is done.

The create_commit docs are available here.

The upload_folder docs are available here.

LysandreJik · 2022-08-01T10:34:38Z

src/transformers/utils/hub.py

+            url = upload_file(
+                path_or_fileobj=os.path.join(working_dir, file),
+                path_in_repo=file,
+                repo_id=repo_id,
+                token=token,
+                commit_message=commit_message,
+            )


If using upload_folder, you won't need to track modified files with their timestamps: if the files have the same name as others on the Hub and contain the same values, they won't get pushed (so if you have local checkpoints that have already been pushed it won't push them again, only verify if they're on the Hub).

LysandreJik · 2022-08-01T10:42:51Z

src/transformers/utils/hub.py

+        modified_files = [
+            f
+            for f in os.listdir(working_dir)
+            if f not in files_timestamps or os.path.getmtime(os.path.join(working_dir, f)) > files_timestamps[f]
+        ]
+        for file in modified_files:
+            if commit_message is None:
+                commit_message = f"Upload {file}."
+            logger.info(f"Uploading {file} to {repo_id}")
+            url = upload_file(
+                path_or_fileobj=os.path.join(working_dir, file),
+                path_in_repo=file,
+                repo_id=repo_id,
+                token=token,
+                commit_message=commit_message,
+            )
+        logger.info(f"{file} was uploaded, you can find it at {url}.")


Suggested change

modified_files = [

f

for f in os.listdir(working_dir)

if f not in files_timestamps or os.path.getmtime(os.path.join(working_dir, f)) > files_timestamps[f]

]

for file in modified_files:

if commit_message is None:

commit_message = f"Upload {file}."

logger.info(f"Uploading {file} to {repo_id}")

url = upload_file(

path_or_fileobj=os.path.join(working_dir, file),

path_in_repo=file,

repo_id=repo_id,

token=token,

commit_message=commit_message,

)

logger.info(f"{file} was uploaded, you can find it at {url}.")

url = upload_folder(

repo_id=repo_id,

folder_path=working_dir,

path_in_repo='.',

commit_message=commit_message, token=token

)

logger.info(f"New version was uploaded, you can find it at {url}.")

LysandreJik · 2022-08-01T10:43:07Z

src/transformers/utils/hub.py

@@ -36,12 +36,13 @@

 import requests
 from filelock import FileLock
-from huggingface_hub import HfFolder, Repository, create_repo, list_repo_files, whoami
+from huggingface_hub import HfFolder, create_repo, list_repo_files, upload_file, whoami


Suggested change

from huggingface_hub import HfFolder, create_repo, list_repo_files, upload_file, whoami

from huggingface_hub import HfFolder, create_repo, list_repo_files, upload_folder, whoami

LysandreJik · 2022-08-01T10:43:28Z

src/transformers/utils/hub.py

+            url = upload_file(
+                path_or_fileobj=os.path.join(working_dir, file),
+                path_in_repo=file,
+                repo_id=repo_id,
+                token=token,
+                commit_message=commit_message,
+            )


Added some comments to use upload_folder instead, it seems like it's an easy enough drop-in replacement

LysandreJik

Thanks for working on this! We'll likely adapt it for the Hub mixin in huggingface-hub.

LGTM!

Wauplin · 2022-08-02T13:22:59Z

src/transformers/modeling_tf_utils.py

+        Upload the model files to the 🤗 Model Hub while synchronizing a local clone of the repo in `repo_path_or_name`.



Small detail: does it make sense to refer to repo_id instead of repo_path_or_name since the later seems to be deprecated ?

Sorry, I just realised it is a merged PR anyway 😬

I can still address ;-)

* Rewrite push_to_hub to use upload_files * Adapt the doc a bit * Address review comments and clean doc

Rewrite push_to_hub to use upload_files

aced457

sgugger requested a review from LysandreJik July 29, 2022 18:42

Adapt the doc a bit

f39f926

julien-c reviewed Jul 29, 2022

View reviewed changes

LysandreJik reviewed Aug 1, 2022

View reviewed changes

Address review comments and clean doc

d1c9242

LysandreJik approved these changes Aug 1, 2022

View reviewed changes

sgugger merged commit 01db72a into main Aug 1, 2022

sgugger deleted the revamp_push_to_hub branch August 1, 2022 16:07

Wauplin reviewed Aug 2, 2022

View reviewed changes

osanseviero mentioned this pull request Aug 3, 2022

Update pinned hhub version #18448

Merged

NielsRogge mentioned this pull request Aug 25, 2022

Update push_to_hub to leverage HTTP API #18299

Closed

oneraghavan pushed a commit to oneraghavan/transformers that referenced this pull request Sep 26, 2022

Rewrite push_to_hub to use upload_files (huggingface#18366)

01f5080

* Rewrite push_to_hub to use upload_files * Adapt the doc a bit * Address review comments and clean doc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite push_to_hub to use upload_files #18366

Rewrite push_to_hub to use upload_files #18366

sgugger commented Jul 29, 2022

HuggingFaceDocBuilderDev commented Jul 29, 2022 •

edited

Loading

julien-c left a comment

julien-c Jul 29, 2022

LysandreJik Aug 1, 2022

LysandreJik Aug 1, 2022

LysandreJik Aug 1, 2022

sgugger Aug 1, 2022

julien-c Aug 1, 2022

LysandreJik Aug 1, 2022

LysandreJik left a comment

LysandreJik Aug 1, 2022

LysandreJik Aug 1, 2022

LysandreJik Aug 1, 2022

LysandreJik Aug 1, 2022

LysandreJik Aug 1, 2022

LysandreJik Aug 1, 2022

LysandreJik Aug 1, 2022

LysandreJik left a comment

Wauplin Aug 2, 2022

Wauplin Aug 2, 2022

sgugger Aug 2, 2022

		>>> pt_model.push_to_hub("my-awesome-model", organization="my-awesome-org")
		>>> pt_model.push_to_hub("my-awesome-org/my-awesome-model")

	from huggingface_hub import HfFolder, create_repo, list_repo_files, upload_file, whoami
	from huggingface_hub import HfFolder, create_repo, list_repo_files, upload_folder, whoami

		Upload the model files to the 🤗 Model Hub while synchronizing a local clone of the repo in `repo_path_or_name`.

Rewrite push_to_hub to use upload_files #18366

Rewrite push_to_hub to use upload_files #18366

Conversation

sgugger commented Jul 29, 2022

What does this PR do?

HuggingFaceDocBuilderDev commented Jul 29, 2022 • edited Loading

julien-c left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LysandreJik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LysandreJik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HuggingFaceDocBuilderDev commented Jul 29, 2022 •

edited

Loading